Looks like we had a merge in master here instead of a rebase, so it's showing a lot more commits. Could you fix that? Thanks!

I had another read of https://webpack.js.org/plugins/environment-plugin/. It either takes an array of string keys, or an object mapping the keys to the default values.

Unfortunately the else clause will violate that. I think the correct course of action to take would be to switch this middleware to always take an object; the bad news: that's a breaking change.

Right now the only thing we can do is leave the plugin as is, and allow consumers to override it if they want to replace the arg. We will have to wait on this PR until we are ready for Neutrino v9. Then we can switch to

module.exports = (neutrino, opts = {}) => {
  neutrino.config
    .plugin('env')
      .use(EnvironmentPlugin, merge({
        NODE_ENV: 'development'
      }, opts);
};

Unfortunately right now we can't move forward. 😢

@zeropaper with master now representing what will be released in v9, would you be willing to pick this up again?

Shouldn't this line be backwards compatible and allow passing an object?

 .use(EnvironmentPlugin, Array.isArray(envs) ? ['NODE_ENV', ...envs] : { 'NODE_ENV': 'development', ...envs });

(see #749)

@ftes i think it should be backward compatible (and my PR reflects it: 4f6b4bf).
Time as passed and I don't have the whole issue in mind anymore but will look at it in the next days (unless you want to take care of it ASAP)

unless you want to take care of it ASAP

Not urgent for me, happy to wait. Let me know if I can be of any more help.

I actually dug a bit and apparently, the use() method is expecting an array:

use(plugin, args = []) {
    return this
      .set('plugin', plugin)
      .set('args', args);
}

the instanciation of the plugin is done in webpack-chain/Plugin.js (line 9)

this.init((Plugin, args = []) => new Plugin(...args)); // it hardly would make sense to me for "args" to be an object

so, unless I miss something.. this PR should be merged as-is.

So there are a few things here that I think made this issue confusing:

• webpack-chain's config.plugin('foo').use(SomePlugin, args) actually expects args to be an array of arguments, that will then be expanded. So if one wants to pass an array as the first argument, the correct form is .use(SomePlugin, [ ['var'] ]), which then internally results in new SomePlugin(['var']).
• Even though the EnvironmentPlugin docs don't mention it, there is actually a third way to pass the environment variable keys: as separate arguments (rather than an array). ie: new EnvironmentPlugin('var', 'another_var').

As such, the current @neutrinojs/env implemention's .use(EnvironmentPlugin, envs) (and the same prior to the webpack 4 changes) is actually using this hidden third form instead of the array form - ie: new EnvironmentPlugin(...envs) rather than new EnvironmentPlugin(envs).

Now that NODE_ENV is no longer set (as of #809), it's much simpler to support both the array and object forms. I also think @neutrinojs/env should be removed and just inlined in @neutrinojs/web.

I'll open a PR for this now, since removing @neutrinojs/env (and any other packages) blocks releasing the first alpha (since we don't want to issue a v9 alpha of any package that won't have a final version release).